Migrate TUI from cursive to ratatui#3905
Conversation
| @@ -0,0 +1,152 @@ | |||
| // Copyright 2024 The Grin Developers | |||
wiesche89
left a comment
There was a problem hiding this comment.
please rebase (merge conflicts)
Replaces the cursive/pancurses-backed TUI with ratatui + crossterm, which has no external C dependency (no more linking libncurses). This fixes cross-compilation failures, macOS crashes on mouse clicks, and Windows crashes on first run reported in mimblewimble#3841. The cursive retained-mode widget tree is replaced with a single App struct (src/bin/tui/app.rs) that every screen renders from each frame. All previous behavior (status fields, peer/mining table columns and formatting, bottom-anchored log view, keybindings, Controller's public API) is preserved, with a few additions: - Mouse support: click a menu item to switch tabs, scroll wheel moves table selection (safe now that ncurses is gone) - PageUp/PageDown/Home/End navigation in tables, with selection clamped to the table's row count - Redraws are throttled: the UI only repaints on input or new data, with a 250ms cap to keep time-based fields fresh Interactive column sorting (cursive_table_view) is dropped since ratatui's Table has no equivalent; tables render in the order ServerStats provides. Fixes mimblewimble#3841
Removes the now-unused cursive/pancurses dependency tree and resolves the ratatui/crossterm graph against the current staging lockfile.
c08dfce to
0f62d93
Compare
wiesche89
left a comment
There was a problem hiding this comment.
Ratatui looks like the right direction and works well on macOS. I couldn’t test Linux or Windows. I left a few comments that I’d prefer to address in this PR so the migration lands cleanly.
| self.cursive.step(); | ||
| if self.needs_redraw || self.last_draw.elapsed() >= MAX_REDRAW_INTERVAL { | ||
| let app = &mut self.app; | ||
| let _ = self.terminal.draw(|f| draw(f, app)); |
There was a problem hiding this comment.
Could we propagate terminal input and render errors instead of dropping them? A failed poll, read, or draw currently leaves the controller running without a usable UI, and a failed draw is still marked as completed.
| if show_dialog_clone.load(Ordering::Relaxed) { | ||
| c.pop_layer(); | ||
| fn handle_key(&mut self, code: KeyCode) { | ||
| if let Some(dialog) = &self.app.dialog { |
There was a problem hiding this comment.
Mouse input is blocked for every dialog, but keyboard input is only blocked for errors. During startup, Tab/j/Enter can still change the view behind the info dialog. Could we make these dialogs modal while still allowing q to shut down?
| for raw_line in text.split('\n') { | ||
| let mut current = String::new(); | ||
| let mut current_len = 0usize; | ||
| for word in raw_line.split(' ') { |
There was a problem hiding this comment.
Could we use Paragraph::wrap() and line_count() here instead of maintaining a separate wrapping implementation? That would preserve terminal display width and whitespace while still allowing the log view to stay bottom aligned.
| state.select(None); | ||
| return; | ||
| } | ||
| let current = state.selected().unwrap_or(0) as i64; |
There was a problem hiding this comment.
TableState starts at None, so the first Down/j/scroll jumps straight to row 1. Could we anchor the first move to row 0 and cover it with a small test?
| GGGGGGGGGGGGG GGGGGGGG GGGGGGGGGGGGG | ||
| GGGGGGGGGGGGGG GGGGGGGG GGGGGGGGGGGGGG | ||
| GGGGGGGGGGGGGG GGGGGGGGGGGGGGGGGGGGGGGGGGGGG | ||
| pub const _WELCOME_LOGO: &str = " GGGGG GGGGGGG |
There was a problem hiding this comment.
After the migration this module only contains the unused _WELCOME_LOGO. Could we remove the file and mod constant, as part of the cleanup?
| pub fn stop(&mut self) { | ||
| self.cursive.quit(); | ||
| self.app.should_quit = true; | ||
| let _ = disable_raw_mode(); |
There was a problem hiding this comment.
The terminal restore sequence is now repeated in the panic hook, stop(), and Drop. Could we centralize it and make the cleanup so these paths cannot drift?
| _ => {} | ||
| }, | ||
| match message { | ||
| UIMessage::UpdateStatus(update) => self.app.stats = Some(update), |
There was a problem hiding this comment.
Now that the UI owns App and status updates happen on the same controller thread, could we update app.stats directly and remove the single-message UI channel? It looks like leftover Cursive plumbing.
| lines.push(line("Stem Pool Size:", "0 (0)")); | ||
| } | ||
| lines.push(Line::from(SEPARATOR)); | ||
| // These three lines are reserved for mining config/status/network info, |
There was a problem hiding this comment.
These fields were never populated in the old view. Could we drop the reserved blank rows instead of carrying the unused placeholders into the new renderer?
|
|
||
| /// Redraw at least this often even without input or new data, so | ||
| /// time-based fields (peer "last seen" ages, etc.) stay fresh. | ||
| const MAX_REDRAW_INTERVAL: Duration = Duration::from_millis(250); |
There was a problem hiding this comment.
The time based fields only change once per second, while 250 ms still rebuilds the full UI four times per second when idle. Could this use a one-second refresh, or is there a measurement supporting the shorter interval?
| let height = area.height as usize; | ||
|
|
||
| LogBufferView { buffer } | ||
| // Walk entries newest-first, wrapping each until we have enough rows to |
There was a problem hiding this comment.
There’s quite a bit of commentary walking through this rendering step by step. Could we simplify it by using Ratatui’s wrapping and line counting support, then keep only the non obvious bottom alignment note?
Summary
cursive/cursive_table_viewTUI (which pulls inpancurses/ncurses) withratatui+crossterm, which have no external C dependencyj/k/arrows,Tab,Enter,Esc,q, plusw/dto toggle the Mining sub-view) are preservedPageUp/PageDown/Home/Endnavigation in tables, and throttled redraws (repaint only on input or new data, 250ms cap) to reduce idle CPUcursive_table_view) is dropped, sinceratatui::widgets::Tablehas no equivalent — tables render in the orderServerStatsprovides. Can be revisited as a follow-up if wanted.Controller::new/Controller::run()'s public API (used bysrc/bin/cmd/server.rs) is unchangedKeyEventKind::Press, avoiding the classic doubled-keystroke issue with crossterm on WindowsTest plan
cargo build --bin grin— 0 errorscargo test --bin grin tui::status— bothupdate_sync_statusunit tests pass unmodifiedcargo clippy --bin grin— zero findings insrc/bin/tuiotool -Lthe built binary has noncurses/pancurseslinkageHome/End/PageUp/PageDown, mouse tab-clicks and wheel scrolling (via SGR escape injection) all verified; error-dialog path renders correctly;qexits cleanly with exit code 0 and the terminal fully restored (raw mode off, alternate screen left, mouse capture disabled)Fixes #3841